-
Notifications
You must be signed in to change notification settings - Fork 943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve #957
base: master
Are you sure you want to change the base?
Resolve #957
Conversation
app/main.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to commit anything to this file
app/cinema.sqlite
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete DB from your PR
self.connection.execute( | ||
f""" | ||
INSERT INTO {self.table_name} (first_name, last_name) | ||
VALUES (?, ?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use parametrization, as here you are sure which fileds you insert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't change this
first_name = ?, last_name = ?
app/managers.py
Outdated
) | ||
self.connection.commit() | ||
|
||
def all(self) -> list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function will return list of what? Fix annotation
app/managers.py
Outdated
Actor(*actor) | ||
for actor in all_actors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do it on one line
app/managers.py
Outdated
SET first_name = ?, last_name = ? | ||
WHERE id = ? | ||
""", | ||
(id_, new_first_name, new_last_name), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the order of parameters
self.connection.execute( | ||
f""" | ||
INSERT INTO {self.table_name} (first_name, last_name) | ||
VALUES (?, ?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't change this
first_name = ?, last_name = ?
app/managers.py
Outdated
|
||
class ActorManager: | ||
def __init__(self) -> None: | ||
self.connection = sqlite3.connect("../cinema.sqlite") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"cinema.sqlite" will be enough
No description provided.